Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat/image library entries #77

Conversation

seandoyle
Copy link
Contributor

Small changes for ImageLibrary entries. The current output passes the pixelmed validator.

TBD - adding modality specific descriptors in Image Library Entries (TID 1602)

@seandoyle seandoyle changed the base branch from master to feat/sr-content-decoding June 2, 2021 18:51
@hackermd hackermd self-requested a review June 2, 2021 20:50
Copy link
Collaborator

@hackermd hackermd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @seandoyle. I defer to your judgment of whether and how we should implement modality-specific descriptors. However, that's something we can address with a separate PR.

Copy link
Collaborator

@hackermd hackermd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just noticed that the tests fail due to style and type issues (https://travis-ci.org/github/MGHComputationalPathology/highdicom/jobs/773313211). @seandoyle you kindly fix those?

@seandoyle
Copy link
Contributor Author

Yes - I'll address those. I also need to add some unit tests.

@seandoyle
Copy link
Contributor Author

The style issues are now fixed but there are other errors in templates.py which I believe are not related to my changes. Should I look at these as well?

BTW:

The DICOM SR validator needs a lot of resources. java -Xmx6g -Xms5g -XX:-UseGCOverheadLimit -cp "${PIXELMEDDIR}/pixelmed.jar:${PIXELMEDDIR}/lib/additional/commons-compress-1.12.jar:${PIXELMEDDIR}/lib/additional/commons-codec-1.3.jar" com.pixelmed.validate.DicomSRValidator $* - 6GB is a lot of memory.

Copy link
Collaborator

@hackermd hackermd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @seandoyle. We can address the other typing errors in the target branch.

@hackermd hackermd merged commit a8158ff into ImagingDataCommons:feat/sr-content-decoding Jun 2, 2021
hackermd added a commit that referenced this pull request Aug 31, 2021
* Integrate new pydicom DS formatting

* Fix recording of evidence in structured reports

* Fix mypy errors

* Add test for report referencing multiple studies

* Add properties to content items

* Use datetime workaround for python 3.6 support

* Add properties and methods on SR templates

* Update type of content item property return value

* Fix typo in exception message

* Fix recording of evidence in structured reports

* Update package version

* Add image library entries to SR documents (#77)

* Fixed enum RelationshipType

* Fix handling of library entry descriptors

* Fix default value for Pixel Origin Interpretation

* Implement indexing by dimension index values

* Rename tracking id methods

* Add check for correct segment numbers

* Fix implementation of content property on SR documents

* Make value and unit required for NUM content items

* Fix return value type of properties of NUM content item

* Add qualifier property to NUM content item

* Return uids of type UID instead of str

* Simplify reshaping of graphic data array

* Add is_root parameter to alternative constructor

* Allow constructor of UID to accept a value

* Chang Optional[x] -> Union[x, None] in docstrings

* Log warning if wrong number of content items

* Add property for temporal range type

* Move QualitativeEvaluation from content to templates

* Allow Concept Name Code Sequence to be absent

* Require Concept Name Code Sequence for CONTAINER

* Let pydicom handle encapsulated frame decoding

* Write error traceback to stderr instead of stdout

* Add docstrings for enums

* Implementation of Parametric Map IOD (#89)

* Changed PatientSexValues to single letters

* Expose enum at package root

* Remove sphinx duplication for highdicom.enum

* Alternative SCImage Constructor (#97)

* Only warn if patient name check fails

Co-authored-by: hackermd <[email protected]>
Co-authored-by: Christopher Bridge <[email protected]>
Co-authored-by: Sean Doyle <[email protected]>
Co-authored-by: Sean Doyle <[email protected]>
Co-authored-by: Markus D. Herrmann <[email protected]>
Co-authored-by: Chris Gorman <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants